-
Notifications
You must be signed in to change notification settings - Fork 140
feat: Add BackendTLS Policy support #1487
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comments, requests and suggestions. I haven't reviewed some unit tests code because of the anticipation they might changed because of the changes to the code they test.
site/content/includes/installation/install-gateway-api-resources.md
Outdated
Show resolved
Hide resolved
site/content/includes/installation/install-gateway-api-resources.md
Outdated
Show resolved
Hide resolved
c0f4672 to
ca31db4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM: specific issues I would have mentioned were already flagged by @pleshakov, so I will avoid redundancy.
Is there a desire for the example to become a documentation how-to use case?
64cae3f to
077a917
Compare
907b0f1 to
a3dcd42
Compare
site/content/how-to/traffic-management/backend-tls-termination.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/backend-tls-termination.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/backend-tls-termination.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/backend-tls-termination.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/backend-tls-termination.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/backend-tls-termination.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/backend-tls-termination.md
Outdated
Show resolved
Hide resolved
65fc3cf to
1bb09fb
Compare
site/content/how-to/traffic-management/backend-tls-termination.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/backend-tls-termination.md
Outdated
Show resolved
Hide resolved
|
@ADubhlaoich will probably want to re-review the new guide that was added. |
b331dc4 to
e7dc901
Compare
e7dc901 to
2b8cbf6
Compare
c7a9fdd to
fb1ac8c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ciarams87
I added a few suggestions and requests in places where readability could be improved.
I also noticed a bug (panic)
otherwise, this looks good to me
bf86b9c to
facbe81
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
c3df833 to
244391d
Compare
* Add BackendTLS Policy support
|
Any idea when this functionality will be supported officially and not through experimental features? |
|
Hey @alex-ivascu, as you may know, BackendTLSPolicy support was promoted to the standard release channel in Gateway api v1.4.0. We have an issue tracking our upgrade to Gateway api v1.4.0, which will go out in our 2.3 release sometime early januaray. |
Proposed changes
Problem: As a user of NGF
I want NGF to implement the BackendTLSPolicy
So that NGF can securely connect to my pods using TLS.
Solution:
Testing: Manual testing, unit testing, conformance testing with and without experimental features enabled (NOTE: no conformance test for Backend TLS policy currently exists - that work is tracked here)
Note: When running the conformance tests with the experimental APIs installed, an unrelated test failed (HTTPRouteInvalidParentRefNotMatchingSectionName). Looks like we have been missing this failure because of a bug in the Gateway API - see https://github.com/nginxinc/nginx-gateway-fabric/actions/runs/7574208095/job/20628086048#step:17:672 for the example output. The fix is to move the check for the existence of a listener for a HTTPRoute above where we check for a port in the ParentRef.
Closes #1262
Checklist
Before creating a PR, run through this checklist and mark each as complete.